Delete dull job and instance models created on no capacity retry#4059
Conversation
| op.execute( | ||
| """ | ||
| UPDATE event_targets SET entity_run_id = entity_id | ||
| WHERE entity_type = 'run' AND entity_run_id IS NULL | ||
| """ | ||
| ) | ||
| op.execute( | ||
| """ | ||
| UPDATE event_targets SET entity_run_id = jobs.run_id | ||
| FROM jobs | ||
| WHERE jobs.id = event_targets.entity_id | ||
| AND event_targets.entity_type = 'job' | ||
| AND event_targets.entity_run_id IS NULL | ||
| """ | ||
| ) |
There was a problem hiding this comment.
Enums like entity_type hold uppercase values
$ sqlite3 ~/.dstack/server/data/sqlite.db "SELECT DISTINCT entity_type FROM event_targets;"
FLEET
GATEWAY
INSTANCE
JOB
PROJECT
RUN
SECRET
USER
VOLUME| if within_fleets is not None: | ||
| query = select(InstanceModel.id).where(InstanceModel.fleet_id.in_(within_fleets)) | ||
| res = await session.execute(query) | ||
| # In Postgres, fetching instance IDs separately is orders of magnitude faster | ||
| # than using a subquery. | ||
| instance_ids = list(res.unique().scalars().all()) | ||
| target_filters.append( | ||
| or_( | ||
| and_( | ||
| EventTargetModel.entity_type == EventTargetType.FLEET, | ||
| EventTargetModel.entity_id.in_(within_fleets), | ||
| ), | ||
| and_( | ||
| EventTargetModel.entity_type == EventTargetType.INSTANCE, | ||
| EventTargetModel.entity_id.in_(instance_ids), | ||
| ), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Event targets now store entity_run_id so that within_runs filter works even for job event when job models are deleted
Should we also introduce entity_fleet_id to be able to see deleted placeholder instance events when using the within_fleets filter?
There was a problem hiding this comment.
No, I think placeholder instance events are not relevant.
There was a problem hiding this comment.
Then should we stop emitting them? Right now there's a weird behavior where the Instance created for job event is emitted and shown in within_fleets. Then, when the instance is deleted, the event disappears from within_fleets (and doesn't disappear if you aren't using the filter).
Although if we stop emitting them, users will still see the placeholder instance in the fleet during provisioning attempts, but won't find any related events, which won't look expected either.
So maybe entity_fleet_id would be the most straightforward fix.
There was a problem hiding this comment.
@jvstme, Should jobs and runs reference entity_fleet_id or only fleets and instances should do that? Do you think we should tie entity_fleet_id semantics to the within_fleets filter specifically?
There was a problem hiding this comment.
Maybe if we'd named entity_* columns something filter-specific like within_filter_* (e.g. within_filter_fleet_id) the semantics of these columns were more clear?
There was a problem hiding this comment.
Should jobs and runs reference
entity_fleet_idor only fleets and instances should do that?
Only fleets and instances.
Do you think we should tie
entity_fleet_idsemantics to thewithin_fleetsfilter specifically?
Well, now entity_fleet_id will only be used in the within_fleets implementation, but the general semantics is that entity_fleet_id connects the target to the fleet that target is part of.
Maybe if we'd named
entity_*columns something filter-specific likewithin_filter_*(e.g.within_filter_fleet_id) the semantics of these columns were more clear?
We may use them for purposes other than filtering in the future. For example, we could return fleet_id in the API, so that callers can see the instance's fleet ID directly in the /api/events/list response, without having to call /api/project/{project_name}/instances/get. Same thing for jobs and their run_id.
The semantics look rather clear to me: entity_project_id -> the target entity is part of that project (or is the project itself), entity_fleet_id -> the target entity is part of that fleet (or is the fleet itself). If there are any doubts, we can explain the semantics in docstrings.
Also note that regular users of the events module won't need to think about these fields at all, since setting them is incapsulated in Target.from_model.
Fixes #4054
Prevent the accumulation of large number of job and instance models when run retries on no capacity:
Add runs to events about potentially never-provisioned jobs so that events are displayed with within_runs filter.Event targets now store entity_run_id so that within_runs filter works even for job event when job models are deleted. entity_run_id is backfilled for existing runs and jobs.Note: the cleanup is for FAILED_TO_START_DUE_TO_NO_CAPACITY jobs that a) carry no useful information and b) created in large quantities on retry. Job models failed due to other reasons are not cleaned up.